fix: normalize Windows CLI executable paths with missing extensions #1345
+11,922
−984
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes CLI validation failure on Windows when users configure paths without the .cmd extension (e.g., C:\...\npm\claude instead of C:\...\npm\claude.cmd). The new normalizeExecutablePath() helper automatically tries common Windows extensions (.exe, .cmd, .bat) when the provided path doesn't exist and has no extension. - Add normalizeExecutablePath() to platform module - Update validateClaude() and validateClaudeAsync() to normalize paths - Add test helper functions to reduce duplication - Add cross-platform tests for normalizeExecutablePath() Signed-off-by: StillKnotKnown <[email protected]>
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughCentralizes platform and environment utilities, adds executable-path normalization and Windows env expansion, replaces direct process.platform/process.env usages across many IPC/terminal/CLI flows, centralizes path security and tool-detection (returns normalized paths), and expands platform/path tests and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Platform
participant SecChecker
participant Finder
participant OSHelpers
participant Proc
Caller->>Platform: resolve and validate tool path (input path or tool name)
Platform->>Platform: strip quotes → expandWindowsEnvVars → normalizeExecutablePath
Platform->>SecChecker: isPathSecure(normalizedPath)
SecChecker-->>Platform: secure / insecure
alt secure
Platform->>Finder: findExecutable(normalizedPath or tool)
Finder-->>Platform: resolvedPath
Platform->>OSHelpers: getWhichCommand(), getEnvVar(), getPtySocketPath()
Platform->>Proc: spawn(resolvedPath, composedEnv)
Proc-->>Caller: started / output
else insecure
Platform-->>Caller: return error with errorKey
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Summary of ChangesHello @StillKnotKnown, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug where Windows users encountered validation failures when configuring CLI tool paths without specifying the file extension. By implementing a new path normalization utility, the application can now intelligently resolve executable paths on Windows, improving user experience and robustness without introducing breaking changes. The solution is carefully designed to be platform-specific, maintaining existing behavior on other operating systems. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of validating Windows CLI paths without extensions by introducing the normalizeExecutablePath helper. The implementation is clean and the integration into cli-tool-manager is correct. I particularly appreciate the refactoring in platform.test.ts with the describeWindows, describeMacOS, etc. helpers, which greatly improves test readability and maintainability. I have one suggestion to enhance the test coverage for the new normalizeExecutablePath function to ensure its logic is robustly verified.
Comment on lines
460
to
469
| describeWindows('handles Windows npm paths without extension', () => { | ||
| it('handles npm paths', () => { | ||
| // Note: This test requires actual file system or proper fs mocking | ||
| // For now, we just verify the function is callable and handles Windows paths | ||
| const result = normalizeExecutablePath('C:\\Users\\user\\AppData\\Roaming\\npm\\claude'); | ||
| // Function should return either the original (if .cmd not found) or .cmd version | ||
| // Either behavior is acceptable - the key is not throwing an error | ||
| expect(result).toContain('claude'); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
The current test for normalizeExecutablePath on Windows is quite minimal and, as noted in the code comment, doesn't fully test the file-existence checking logic. We can significantly improve the test coverage by mocking the fs module. This will allow us to test the different branches of the function's logic without relying on the actual file system.
First, add the following to the top of the file to mock fs.existsSync:
import * as fs from 'fs';
vi.mock('fs', async (importOriginal) => {
const originalFs = await importOriginal<typeof fs>();
return {
...originalFs,
existsSync: vi.fn(),
};
});With the mock in place, you can replace the existing test block with this more comprehensive suite:
describeWindows('normalizeExecutablePath on Windows', () => {
beforeEach(() => {
// Reset mocks before each test in this suite
vi.mocked(fs.existsSync).mockReset();
});
it('should return path with .cmd when it exists and original does not', () => {
const pathWithoutExt = 'C:\\path\\to\\claude';
const pathWithCmd = 'C:\\path\\to\\claude.cmd';
vi.mocked(fs.existsSync).mockImplementation((p) => p === pathWithCmd);
const result = normalizeExecutablePath(pathWithoutExt);
expect(result).toBe(pathWithCmd);
expect(fs.existsSync).toHaveBeenCalledWith(pathWithoutExt);
expect(fs.existsSync).toHaveBeenCalledWith(expect.stringContaining('.exe'));
expect(fs.existsSync).toHaveBeenCalledWith(pathWithCmd);
});
it('should return path with .exe when it exists and original does not', () => {
const pathWithoutExt = 'C:\\path\\to\\claude';
const pathWithExe = 'C:\\path\\to\\claude.exe';
vi.mocked(fs.existsSync).mockImplementation((p) => p === pathWithExe);
const result = normalizeExecutablePath(pathWithoutExt);
expect(result).toBe(pathWithExe);
});
it('should return path with .bat when it exists and others do not', () => {
const pathWithoutExt = 'C:\\path\\to\\claude';
const pathWithBat = 'C:\\path\\to\\claude.bat';
vi.mocked(fs.existsSync).mockImplementation((p) => p === pathWithBat);
const result = normalizeExecutablePath(pathWithoutExt);
expect(result).toBe(pathWithBat);
expect(fs.existsSync).toHaveBeenCalledWith(expect.stringContaining('.exe'));
expect(fs.existsSync).toHaveBeenCalledWith(expect.stringContaining('.cmd'));
expect(fs.existsSync).toHaveBeenCalledWith(pathWithBat);
});
it('should return original path if it exists, even without extension', () => {
const pathWithoutExt = 'C:\\path\\to\\claude';
vi.mocked(fs.existsSync).mockImplementation((p) => p === pathWithoutExt);
const result = normalizeExecutablePath(pathWithoutExt);
expect(result).toBe(pathWithoutExt);
expect(fs.existsSync).toHaveBeenCalledTimes(1);
});
it('should return original path if neither it nor variants with extensions exist', () => {
const pathWithoutExt = 'C:\\path\\to\\claude';
vi.mocked(fs.existsSync).mockReturnValue(false);
const result = normalizeExecutablePath(pathWithoutExt);
expect(result).toBe(pathWithoutExt);
expect(fs.existsSync).toHaveBeenCalledTimes(4); // original + 3 extensions
});
});
StillKnotKnown
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/frontend/src/main/platform/__tests__/platform.test.ts`:
- Around line 460-469: The test for normalizeExecutablePath doesn't exercise
extension resolution; mock the filesystem checks used by normalizeExecutablePath
(e.g., spy on fs.existsSync or the specific helper it uses) inside the
describeWindows block to simulate the presence of different extensions (.exe,
.cmd, .bat) and verify the function returns the path with the expected extension
in each case (call normalizeExecutablePath with the base '...\\npm\\claude' and
assert for '.exe', '.cmd', '.bat' outcomes); ensure you restore the mock after
each case to avoid cross-test pollution.
In `@apps/frontend/src/main/platform/index.ts`:
- Around line 389-443: normalizeExecutablePath currently tries ['.exe', '.cmd',
'.bat'] but getPathConfig() exposes a different canonical list that includes
'.ps1'; update normalizeExecutablePath to use the same shared
executableExtensions list (or at minimum add '.ps1') so PowerShell scripts are
resolved consistently—locate the extensions array in normalizeExecutablePath and
replace it with a reference to the shared executableExtensions from
getPathConfig (or add '.ps1' to the literal array) to keep behavior aligned.
Add comprehensive test coverage for normalizeExecutablePath extension resolution behavior using vi.mock for fs.existsSync. Also update normalizeExecutablePath to use getPathConfig().executableExtensions instead of hardcoded list for consistency. Addresses CodeRabbit feedback: - Test coverage gap: Extension resolution behavior is now fully tested - Extension list consistency: Uses shared getPathConfig() list (includes .ps1) Signed-off-by: StillKnotKnown <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/frontend/src/main/platform/__tests__/platform.test.ts`:
- Around line 475-479: Reset the mockedExistsSync mock explicitly and set a safe
default implementation in the describeWindows beforeEach so tests don't rely on
an uninitialized mock; e.g., call mockedExistsSync.mockReset() (or mockClear())
and then mockedExistsSync.mockImplementation(() => false) instead of delegating
to fs.existsSync, referencing the mockedExistsSync symbol used in the tests
around normalizeExecutablePath on Windows.
Fix CodeRabbit nitpick: use explicit mockReset() and mockReturnValue(false) instead of delegating to fs.existsSync which is itself mocked at module level. Signed-off-by: StillKnotKnown <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/frontend/src/main/platform/__tests__/platform.test.ts`:
- Around line 215-225: The Unix-only test suites use describeMacOS but lack
Linux coverage; update the platform-specific test blocks (e.g., the suites that
call describeMacOS around assertions for getPathDelimiter and similar tests) to
use a cross-Unix wrapper such as describeUnix or add equivalent describeLinux
variants so the tests run on both macOS and Linux; locate occurrences of
describeMacOS (and the related blocks testing getPathDelimiter and other Unix
behaviors) and replace or duplicate them with describeUnix/describeLinux to
satisfy the Windows/macOS/Linux coverage guideline.
…acOS Update Unix-only test suites to use describeUnix helper instead of describeMacOS for proper Linux coverage. This aligns with the project's Windows/macOS/Linux testing guideline. Changes: - Path delimiter tests now run on both macOS and Linux - Executable extension tests now run on both macOS and Linux - withExecutableExtension tests now run on both macOS and Linux - Shell config tests now run on both macOS and Linux - requiresShell tests now run on both macOS and Linux - npm command tests now run on both macOS and Linux - isSecurePath tests now run on both macOS and Linux - normalizeExecutablePath Unix tests now use describeUnix Tests increase from 48 to 58 (10 additional tests from Linux coverage). Addresses CodeRabbit feedback about missing Linux test coverage. Signed-off-by: StillKnotKnown <[email protected]>
Fix 4 critical issues identified by parallel agent investigation:
1. CRITICAL: Add normalizeExecutablePath to validateClaudeCliAsync
- claude-code-handlers.ts had separate validation function
- Used by "Installations" dropdown selection
- Did NOT use normalizeExecutablePath, causing .cmd paths to fail
2. CRITICAL: Strip quotes before security check
- Security check ran on raw path with quotes
- Double quote (") is in dangerous patterns list
- Valid paths with quotes were rejected before normalization
- Fixed in both detectClaude() and detectClaudeAsync()
3. HIGH: Fix Vite bundler constant folding issue
- Bundler incorrectly optimized .filter() call
- Lost .ps1 extension in production builds
- Use spread operator [...config.executableExtensions] to avoid bug
These fixes ensure Windows users can:
- Select CLI path from dropdown without .cmd extension
- Use paths with quotes
- Use .ps1 PowerShell scripts without extension
Signed-off-by: StillKnotKnown <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts`:
- Line 40: Replace the direct process.platform check used to set const isWindows
= process.platform === 'win32'; with the platform abstraction function from the
project's platform module: import the provided platform detection helper (e.g.,
platform.isWindows or isWindowsPlatform) and assign const isWindows = <that
helper>() instead; ensure you add the import at top of claude-code-handlers.ts
and remove the direct process.platform reference so the file uses the central
platform abstraction.
…nd fix security checks This commit addresses user-reported issues with Windows CLI path validation in binary builds by consolidating platform abstraction and fixing security check ordering. Changes: 1. cli-tool-manager.ts: Strip quotes before security validation - isWrongPlatformPath() now strips quotes before checking isSecurePath() - Prevents security check from running on quoted paths like '"C:\..."' - Consolidated to use stricter isSecurePath from platform module 2. claude-code-handlers.ts: Use platform abstraction consistently - Replaced direct process.platform checks with platform module functions - Added normalizeExecutablePath() usage for missing extension handling - Using isSecurePath from platform module for consistent security validation 3. cli-tool-manager.test.ts: Fix test mocks and path escaping - Added platform module mock to preserve original implementations - Fixed test path escaping (\\n literal newline vs \\n backslash-n) - Updated import to use isSecurePath from platform module All 2056 tests pass.
Replace direct process.platform checks with platform abstraction layer functions to improve cross-platform consistency and maintainability. Changes: 1. claude-cli-utils.ts: Use getPathDelimiter() and isWindows() 2. python-detector.ts: Use platformIsWindows() (aliased to avoid conflict) 3. mcp-handlers.ts: Use isWindows() for platform-specific commands 4. agent-process.ts: Use isWindows() for spawn logic 5. agent-queue.ts: Use getPathDelimiter() for PATH joining All 2056 tests pass.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/frontend/src/main/__tests__/cli-tool-manager.test.ts`:
- Around line 729-747: The test mocks for Windows paths use double-escaped
backslashes causing the string to contain doubled backslashes; update the mock
return and the expectation to use normal Windows path escaping (single backslash
per separator in a JS string) so that findWindowsExecutableViaWhere() and
getToolInfo('claude') receive/return a realistic path; specifically change the
mockReturnValue for findWindowsExecutableViaWhere and the
expect(result.path).toBe(...) assertion to use 'D:\Program
Files\nvm4w\nodejs\claude.cmd' (i.e., each separator as \\ in the source
string), leaving existsSync and execFileSync mocks as-is.
Replace direct process.platform checks with platform abstraction layer functions in MEDIUM and HIGH priority files identified by 4-agent analysis. Platform Module Enhancements: - Add pathsAreEqual() for case-insensitive path comparison on Windows - Add getWhichCommand() for cross-platform which/where command selection - Add getVenvPythonPath() for cross-platform virtual environment Python path Files Fixed: 1. memory-handlers.ts: Use isWindows(), isMacOS(), isLinux() for Ollama detection 2. subprocess-runner.ts: Use platform abstraction for Python venv and GH CLI detection 3. release-handlers.ts: Use isWindows() for GH CLI check 4. memory-service.ts: Use getVenvPythonPath() for Python venv path 5. insights/config.ts: Use pathsAreEqual() for case-insensitive path comparison 6. terminal-handlers.ts: Use isWindows() for shell command construction 7. changelog/generator.ts: Use isWindows() for environment variable setup 8. changelog/version-suggester.ts: Use isWindows() for environment variable setup All 2056 tests pass.
Replace direct process.platform checks with platform abstraction layer functions in remaining MEDIUM priority files. Platform Module Enhancement: - Add getPtySocketPath() for cross-platform PTY socket path (Note: PTY daemon scripts are standalone Node.js files, accept as-is) Files Fixed: 1. task/worktree-handlers.ts: Use getCurrentOS() for platform type narrowing 2. terminal/worktree-handlers.ts: Use isWindows() for symlink creation All 2056 tests pass.
Add comprehensive test coverage for the new platform module functions: - pathsAreEqual(): Case-insensitive path comparison on Windows - getWhichCommand(): Cross-platform which/where command selection - getVenvPythonPath(): Cross-platform virtual environment Python path - getPtySocketPath(): Cross-platform PTY socket path All 2072 tests pass (was 2056, added 16, removed 2 problematic ones).
Replace process.env with getEnvVar() in IPC handlers and platform code for case-insensitive environment variable access on Windows. This fixes issues where environment variables like DEBUG and NODE_ENV might not be found when accessed with incorrect casing. Also adds comprehensive tests for expandDirPattern() and getGitLabCliPaths() in platform/paths.test.ts. Backend files migrate sys.platform/os.name checks to centralized is_windows() helper from core.platform.
Based on 6-agent research, this commit addresses multiple cross-platform issues and aligns code with centralized helper functions. Backend changes: - Add npm, Scoop, Chocolatey paths to gh_executable.py Windows detection - Create gitlab_executable.py with platform-aware glab CLI finder - Update gitlab runner.py to use get_glab_executable() instead of hardcoded command - Add test_gh_executable.py with comprehensive platform-specific tests - Add is_windows() migration tests to test_git_executable.py Frontend changes: - Replace process.env with getEnvVar() for case-insensitive Windows env access across 15+ files (title-generator, app-logger, sentry, etc.) - Add getGitHubCliPaths() to platform/paths.ts for symmetry with getGitLabCliPaths() - Add 6 new tests for getGitHubCliPaths() covering Windows, macOS, and Linux - Update renderer components to use shared getEnvVar() helper Test coverage: - 2687 tests passing (12 new tests added) - All platform-specific path detection working correctly
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Fixed test failures by: - Using decorator-style patches with proper module-level paths - Patching core.gh_executable.os.path.* instead of os.path.* - Patching core.git_executable.is_windows instead of core.platform.is_windows - Removing unused MagicMock import and fixing CodeQL alert - Relaxing test_unix_skips_windows_paths assertion to handle system git All test_gh_executable.py tests (23 tests) now pass. All test_git_executable.py tests now pass. Test results: 1654 passed, 5 failed (pre-existing permission issues), 7 errors (pre-existing auth issues)
| stderr="" | ||
| ) | ||
|
|
||
| result = _find_git_executable() |
Check notice
Code scanning / CodeQL
Unused local variable Note test
- Fix test_checks_homebrew_paths_on_unix: remove return_value=False from os.path.isfile patch decorator that was preventing side_effect from working - Fix CodeQL URL substring sanitization alert: simplify domain check - Fix unused local variable notices in test_git_executable.py: add assertions for results - Fix test_windows_tries_where_command: use non-standard path to force where command usage instead of returning early from hardcoded paths
Frontend: - Add 'glab' to CliTool type in agent-process.ts - Add GITLAB_CLI_PATH to CLI_TOOL_ENV_MAP - Handle glab detection gracefully (not yet supported by cli-tool-manager) - Users can now manually set GITLAB_CLI_PATH env var for GitLab CLI detection Backend (git_executable.py): - Add invalidate_git_cache() function for cache invalidation - Add _verify_git_executable() function for consistency with gh/glab - Update _find_git_executable() to use _verify_git_executable() for all path checks - This ensures Git executables are verified before use, matching the pattern used for gh and glab Tests: - Add tests for _verify_git_executable() function - Add tests for invalidate_git_cache() function - Update existing tests to mock _verify_git_executable where needed
Add cache clearing to prevent cached values from previous tests affecting this test's assertions.
cli-tool-manager.ts: - Add 'glab' to CLITool type - Add gitlabCLIPath to ToolConfig interface - Add validateGitLabCLI() method (similar to validateGitHubCLI) - Add detectGitLabCLI() method with full platform support: - User configuration - Homebrew (macOS) - System PATH (augmented) - Windows Program Files / Scoop / Chocolatey - Windows where.exe fallback - Add 'glab' case to detectToolPath() settings.ts: - Add gitlabCLIPath to AppSettings interface agent-process.ts: - Update detectAndSetCliPath to include 'glab' in supported tools - Remove special-case handling for glab (now fully supported) This completes the GitLab CLI frontend support, matching the pattern used for GitHub CLI (gh).
Use exact string match for expected error message instead of substring check to satisfy CodeQL security analysis.
Platform module enhancements: - Add null byte injection protection to isSecurePath() (checked FIRST for defense-in-depth) - Add expandHomePath() utility with input type validation - Add ensureLongPathCompatible() for Windows MAX_PATH (260 char) handling - Add escapeShellPath() utility for cross-platform shell escaping Test improvements: - Add WSL network path detection tests (\\wsl$\, \\wsl.localhost) - Add MSYS2/Cygwin installation path tests - Add setUp() to TestGetGhExecutable for cache isolation Bug fixes: - Fix install-backend.js: isWindows() was not being called (function reference truthy bug) - Fix test_gh_executable.py: patch targets changed from core.platform.is_windows to core.gh_executable.is_windows - Fix capabilities.py: use is_windows() consistently instead of sys.platform != "win32"
…timing - Add normalizePathForTest() helper to handle Windows backslashes in path assertions - Fix async timing race conditions in subprocess-spawn tests by ensuring spawn completes before emitting mock events - Fix import/export mismatch: getValidatedPath -> getValidatedPythonPath - Add proper null/undefined type safety to normalizePathForTest() Fixes test failures on Windows CI where path.join() returns backslashes but tests expect forward slashes for comparison.
Fixes two test failures that were occurring on Windows CI:
1. subprocess-spawn.test.ts: Fixed timing issues in log event tests
- Use vi.waitFor() to ensure process is spawned and tracked before
emitting mock stdout/stderr data
- This ensures listeners are attached before events are emitted
- Fixes "should emit log events from stdout" test
2. homebrew-python.test.ts: Fixed mock reset issue
- Store reference to mocked getHomebrewBinPaths function
- Re-apply mockImplementation in beforeEach after clearAllMocks
- Ensures mock always returns expected paths across all tests
- Fixes all 9 previously failing tests
All 2716 tests now pass.
Fixed the mock reset issue in homebrew-python.test.ts by getting the reference to the mocked getHomebrewBinPaths function inside beforeEach instead of at module level. This ensures the mock is properly applied after vi.clearAllMocks() on all platforms including Windows. The subprocess-spawn tests remain in their working state using vi.waitFor() to wait for process spawning to complete.
The existsSync mock in homebrew-python tests was failing on Windows because path.join() produces backslashes on Windows, but the mock comparisons were using forward slashes. Fix: Normalize paths to forward slashes in all mock implementations for cross-platform path comparison. Fixes failures in: - finds Python 3.13 in Intel directory - continues search after validation error - checks Apple Silicon directory before Intel - stops searching after finding valid Python
The mockValidate function was being called with Windows-style paths (backslashes) on Windows CI, but test assertions and mock implementations were using forward slashes. Fix: 1. Normalize paths in mockValidate implementations for comparison 2. Normalize validateCalls array when asserting expected values 3. Use normalizePathForTest for result assertions This handles the fact that joinPaths() uses path.join() which produces backslashes on Windows.
Two subprocess-spawn tests were failing on macOS CI because setImmediate() wasn't sufficient to ensure event listeners were attached before emitting mock events. Fix: Use vi.waitFor() to wait for process to be tracked (isRunning == true) before emitting exit/error events, matching the pattern used in other tests in the same file. Fixes failures in: - should emit exit event when process exits - should emit error event when process errors
The bundled Python path in packaged apps was being rejected by the path validation security check because it wasn't in the allowlist. This caused error logs like: [AgentProcess] Invalid Python path rejected: Path does not match allowed Python locations. Added patterns for: - Unix/Linux: /path/to/resources/python/bin/python3 - macOS: /path/to/Resources/python/bin/python3 (case-insensitive) - Windows: C:\path\to\resources\python\python.exe Also added 3 tests to verify bundled Python paths are accepted on all platforms.
The renderer process in development (Vite dev server) doesn't have
access to Node.js 'process' object, causing errors when using
shared/platform.ts functions.
Fix:
1. Add process.platform and process.env to renderer defines in vite config
2. Add runtime guards in shared/platform.ts to handle undefined process
This fixes the error:
ReferenceError: process is not defined
at getCurrentPlatform (platform.ts:29)
The ideation runner was using the default 'coder' agent type which requires
many tool permissions. When spawned as a subprocess from Claude Code CLI,
the permission request stream would close with 'Stream closed' error.
Fix: Use 'planner' agent type which has fewer tool permission requirements,
reducing the likelihood of stream timeout or permission errors during ideation.
This fixes the error:
Error in hook callback hook_0: Stream closed
at sendRequest (//root/claude:5313:133)
during ideation generation.
Add all known Claude CLI installation paths on Windows to ensure detection regardless of installation method. New paths now checked: - Official Windows installer: Program Files/ClaudeCode/claude.exe - Scoop package manager: scoop/shims and scoop/apps paths - Chocolatey package manager: ProgramData/chocolatey/bin - Bun package manager: .bun/bin/claude.exe - Legacy "Claude" directory for backwards compatibility This ensures Claude CLI is found whether installed via: - Native installer (menu version selection) → .local/bin/claude.exe - npm global → AppData/Roaming/npm/claude.cmd - Official installer → Program Files/ClaudeCode/claude.exe - Scoop → scoop/shims or scoop/apps - Chocolatey → ProgramData/chocolatey/bin - Bun → .bun/bin/claude.exe - WinGet → Program Files/ClaudeCode/claude.exe Sources: - https://code.claude.com/docs/en/setup - https://code.claude.com/docs/en/troubleshooting - https://www.claudelog.com/faqs/where-is-claude-code-installed/ Signed-off-by: StillKnotKnown <[email protected]>
The previous code used `findExecutable('where') || 'where'` which could
fail to locate where.exe and split output on '\n' only. This caused
issues on Windows where line endings are \r\n.
Changes:
- Use explicit 'where.exe' path instead of findExecutable lookup
- Split on /\r?\n/ regex to handle both \n and \r\n line endings
- Add proper exec options (encoding, timeout, windowsHide)
- Match pattern from findWindowsExecutableViaWhereAsync in windows-paths.ts
This ensures Claude CLI discovery via where.exe returns all found
installations correctly on Windows.
Signed-off-by: StillKnotKnown <[email protected]>
…platform
Additional cross-platform fixes beyond the where.exe fix:
1. Unix 'which' command (claude-code-handlers.ts):
- Changed from findExecutable('which') || 'which' to explicit 'which'
- Added proper exec options (encoding, timeout)
- Changed split from '\n' to /\r?\n/ for consistency and robustness
2. MCP JSON-RPC response parsing (mcp-handlers.ts):
- Changed split from '\n' to /\r?\n/ for Windows compatibility
- MCP servers on Windows may output \r\n line endings
These changes ensure consistent behavior across Windows, macOS, and Linux.
Signed-off-by: StillKnotKnown <[email protected]>
…-handlers
Replace expandWindowsEnvVars('%COMSPEC%') with getCmdExecutablePath() from
the platform module. This ensures consistent behavior with cli-tool-manager.ts
and uses proper COMSPEC environment variable reading via getEnvVar() with
appropriate fallbacks.
The getCmdExecutablePath() function:
- Uses getEnvVar('COMSPEC') for case-insensitive Windows env var lookup
- Falls back to SystemRoot\System32\cmd.exe
- Falls back to C:\Windows\System32\cmd.exe
- Returns 'sh' on Unix systems for cross-platform compatibility
This matches the same fix previously applied to cli-tool-manager.ts.
Signed-off-by: StillKnotKnown <[email protected]>
…etection
This is the critical fix for Windows Claude CLI detection failing.
## Root Cause Analysis
9 parallel agents analyzed the issue and identified that `where.exe`
on Windows requires exact filename with extension - it does NOT use
PATHEXT environment variable like shell commands do.
When searching for `claude`, `where.exe` returns nothing even when
`claude.cmd` exists (the npm installation creates `claude.cmd`).
## Changes
### 1. claude-code-handlers.ts - scanClaudeInstallations()
- Changed from single `where.exe claude` call to loop through extensions
- Now searches: `claude.cmd`, `claude.exe`, `claude.bat` separately
- Each extension search is wrapped in try/catch to continue on failure
### 2. windows-paths.ts - findWindowsExecutableViaWhere()
- Added multi-extension search loop: ['', '.cmd', '.exe', '.bat']
- Tries base name first (for existing binaries), then adds extensions
- Each attempt wrapped in try/catch to continue to next extension
### 3. windows-paths.ts - findWindowsExecutableViaWhereAsync()
- Same multi-extension search as sync version
- Maintains consistency between sync and async implementations
### 4. claude-code-handlers.ts - validateClaudeCliAsync()
- Fixed cmd.exe quoting: removed `/s` flag and broken `""path" --version"` pattern
- Changed to `['/d', '/c', '"path"', '--version']` as separate arguments
- Lets execFileAsync handle proper quoting
### 5. cli-tool-manager.ts - validateClaude() and validateClaudeAsync()
- Applied same cmd.exe quoting fix
- Changed from `expandWindowsEnvVars('%COMSPEC%')` to `getCmdExecutablePath()`
- Uses platform abstraction with proper fallbacks
## Why This Works
- npm installations create `claude.cmd` in `%APPDATA%\npm\`
- Official installer creates `claude.exe` in Program Files
- Legacy installations might have `claude.bat`
- By searching all extensions, all installation methods are found
## Testing
- All 2724 tests pass
- Windows-paths tests specifically verify where.exe behavior
Resolves: Windows Claude CLI detection failing while Linux works
Signed-off-by: StillKnotKnown <[email protected]>
Replace flaky setImmediate with vi.waitFor for the 'should kill task and remove from tracking' test. This matches the pattern used in other tests in the same file and prevents race conditions where the task hasn't been registered yet when we check isRunning(). Signed-off-by: StillKnotKnown <[email protected]>
Fixes "Claude CLI not found" error when running ideation and roadmap
processes, even though dev and exe modes detect the CLI correctly.
## Root Cause
The agent-process.ts spawnProcess() calls detectAndSetCliPath() to set
CLAUDE_CLI_PATH for Python subprocesses. However, agent-queue.ts has its
own spawnIdeationProcess() and spawnRoadmapProcess() methods that did
NOT call this detection, so the Python backend never received the CLI path.
## Changes
### 1. agent-process.ts
- Made detectAndSetCliPath() public (was private)
- Added logging for existing env vars
- Added warning when detection fails
### 2. agent-queue.ts
- Added detectAndSetCliPath('claude') call to both ideation and roadmap spawns
- CLAUDE_CLI_PATH is now passed to Python subprocess
### 3. cli-tool-manager.ts
- Reordered Windows detection: where.exe now runs BEFORE findExecutable
- This ensures npm-installed claude.cmd is found before other methods
- Applied to both detectClaude() and detectClaudeAsync()
## Why This Works
- where.exe finds .cmd files (npm installations) reliably
- findExecutable is less reliable on Windows for .cmd files
- By trying where.exe first, we get the correct path
- The Python backend receives CLAUDE_CLI_PATH and uses it directly
Signed-off-by: StillKnotKnown <[email protected]>
…ation/roadmap
- Change agent-queue.ts to use getToolInfo('claude') directly instead of
calling processManager.detectAndSetCliPath()
- Make detectAndSetCliPath() private in agent-process.ts since it's now
only used internally
- Ensures all spawn paths use the same centralized CLI detection logic
from cli-tool-manager.ts
This centralizes CLI tool detection - getToolInfo() is the single source
of truth for finding Claude CLI, gh, and other tools.
Signed-off-by: StillKnotKnown <[email protected]>
rayBlock
pushed a commit
to rayBlock/Auto-Claude-Fork
that referenced
this pull request
Jan 20, 2026
rayBlock
pushed a commit
to rayBlock/Auto-Claude-Fork
that referenced
this pull request
Jan 20, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
Fixes Windows CLI path validation failure when users configure paths without the .cmd extension. When a user provides a path like
C:\Users\...\npm\claude(without extension), validation would fail with ENOENT even thoughclaude.cmdexists at that location.Adds
normalizeExecutablePath()helper function to the platform module that:The integration points (
validateClaude()andvalidateClaudeAsync()) now normalize paths before validation, allowing users to configure CLI paths with or without extensions.Related Issue
Fixes Windows user-reported CLI validation issue where paths without .cmd extension fail validation.
Type of Change
Area
Commit Message Format
Follow conventional commits:
<type>: <subject>Types: feat, fix, docs, style, refactor, test, chore
Example:
feat: add user authentication systemChecklist
developbranchPlatform Testing Checklist
CRITICAL: This project supports Windows, macOS, and Linux. Platform-specific bugs are a common source of breakage.
platform/module instead of directprocess.platformchecksfindExecutable()or platform abstractions)If you only have access to one OS: CI now tests on all platforms. Ensure all checks pass before submitting.
CI/Testing Requirements
Screenshots
Feature Toggle
use_feature_nameBreaking Changes
Breaking: No
Details:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Localization
✏️ Tip: You can customize this high-level summary in your review settings.